-
-
Notifications
You must be signed in to change notification settings - Fork 451
Add options for Logs #4374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/log-rate-limit
Are you sure you want to change the base?
Add options for Logs #4374
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Left some comments to be tackled in a later pr
@@ -150,6 +152,9 @@ public final class ExternalOptions { | |||
options.setCaptureOpenTelemetryEvents( | |||
propertiesProvider.getBooleanProperty("capture-open-telemetry-events")); | |||
|
|||
options.setEnableLogs(propertiesProvider.getBooleanProperty("logs.enabled")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also be experimental
@@ -25,6 +25,7 @@ public final class ExternalOptions { | |||
private @Nullable Boolean enableDeduplication; | |||
private @Nullable Double tracesSampleRate; | |||
private @Nullable Double profilesSampleRate; | |||
private @Nullable Double logsSampleRate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be removed as we don't sample logs
* 1.0. if 1.0 is set it means that 100% of items are sent. If set to 0.1 only 10% of items will | ||
* be sent. Items are picked randomly. Default is 1.0 | ||
*/ | ||
@ApiStatus.Experimental private @Nullable Double sampleRate = 1.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be removed
📜 Description
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps
sampleRate
for Logs again as there's some discussion around whether this is actually useful in its current state where it just randomly drops log events without considering log level or the trace it belongs to.